Skip to content

fix(byok): preserve credential ownership on lookup failures - #9054

Merged
waleedlatif1 merged 2 commits into
stagingfrom
codex/byok-fail-closed
Oct 15, 2026
Merged

fix(byok): preserve credential ownership on lookup failures#9054
waleedlatif1 merged 2 commits into
stagingfrom
codex/byok-fail-closed

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

A failed BYOK database lookup could be treated as an absent key, allowing a request to run and be billed with a platform credential. The shared resolver now returns null only after successfully confirming that no eligible customer key applies. Lookup failures and unusable configured key pools stop execution.

Transient database reads use the existing error classifier, jittered backoff, and deadline helpers: at most three attempts within five seconds, with caller cancellation propagated. Tool execution no longer swallows credential failures. Copilot's enterprise gate uses the billing layer's existing strict-read policy so an unavailable subscription lookup cannot bypass BYOK.

Type of Change

  • Bug fix

Testing

  • Seven real-Postgres regressions cover a terminated connection and recovery, exhausted retries, permanent query errors, corrupt ciphertext, confirmed absence, and organization/personal entitlement read failures. Reverting each safeguard makes its regression fail.
  • Focused tests cover cancellation, late database results, and stopping tool dispatch and metering after credential failure.
  • Workspace type checks, test-type ratchet, lint, all 60 audits, and the staging-sensitive block registry check pass.
  • Full repository gate passes: 606 script tests, 37,711 app tests (7 skipped), and all 20 workspace tasks. Ran with bun run test --concurrency=2 -- --maxWorkers=4.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced

@vercel

vercel Bot commented Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 14, 2026 11:52pm UTC

Request Review

@icecrasher321
icecrasher321 marked this pull request as ready for review October 14, 2026 23:36
cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed Oct 14, 2026
View reviewed changes

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 13 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1 waleedlatif1 mentioned this pull request Oct 14, 2026
Merged
@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Critical impact] This PR appears safe to merge; no actionable issues remain.

Summary

This PR stops failed BYOK lookups from silently switching requests to platform credentials.

  • BYOK lookup failures no longer become platform-key requests.
  • Tool requests stop when their BYOK lookup fails.
  • Copilot waits for a clear enterprise answer before using BYOK.

No new actionable issues were found. Tests were inspected, not run.

Diagram

 
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve credential] --> B[Read customer key pool]
  B -->|Transient error| C[Retry within deadline]
  C --> B
  B -->|Permanent error or retries exhausted| D[Stop request]
  B -->|Eligible keys found| E[Decrypt customer key]
  E -->|No usable key| D
  E -->|Success| F[Use customer credential]
  B -->|Confirmed no eligible key| G[Allow normal credential fallback]
Loading

Reviews (2) · Last reviewed commit: "fix(byok): isolate enterprise entitlemen..." · Reviewed by Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Oct 14, 2026
View reviewed changes
Comment thread apps/sim/lib/billing/core/subscription.integration.ts
@icecrasher321
icecrasher321 force-pushed the codex/byok-fail-closed branch from f419a55 to 8dd6810 Compare October 14, 2026 23:52
@icecrasher321

icecrasher321 commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

icecrasher321 commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed Oct 14, 2026
View reviewed changes

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 13 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit c6a00fc into staging Oct 15, 2026
44 checks passed
@waleedlatif1
waleedlatif1 deleted the codex/byok-fail-closed branch October 15, 2026 00:00

This branch was previously deployed

1 inactive deployment
Preview — 8dd68105 Deployed Oct 14, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@icecrasher321 @waleedlatif1